-
Notifications
You must be signed in to change notification settings - Fork 595
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: introduce cluster limit #18383
Conversation
|
Please remember to add a notice on the release note. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM for the rest
message ClusterLimit { | ||
oneof limit { | ||
ActorCountPerParallelism actor_count = 1; | ||
// TODO: limit DDL using compaction pending bytes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure they are either-or relation?
self.env.opts.actor_cnt_per_worker_parallelism_soft_limit, | ||
self.env.opts.actor_cnt_per_worker_parallelism_hard_limit, | ||
), | ||
Ok(Tier::Free) => ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems still a bit controversial to decide whether & how we should set a different limit for free users. Thus, I tend to use the same limit for both paid and free users for now until we have clearer ideas in the future.
This idea also applies to bypass_cluster_limits
, which is now unable to be modified by free users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1. I have updated the PR to unify the behavior for different tiers.
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Related discussion: #16092
To ensure smoother user experience when cluster resource is insufficient, this PR introduces cluster limit in kernel. At the 1st stage, the cluster limit only contains limit on actor count per worker parallelism and there are a lot of headroom for the default limit. The behavior is:
When the soft limit is hit, CREATE MV/TABLE/SINK can succeed with a notice message attached.
When the hard limit is hit, CREATE MV/TABLE/SINK cannot succeed with a notice message attached.
Example:
Checklist
./risedev check
(or alias,./risedev c
)Documentation
Release note
A limit on actor count per parallelism (i.e. CPU core) is added to warn user on
CREATE MATERIALIZED VIEW/TABLE/SINK
statements to ensure smoother user experience when cluster resource is insufficient. Both a soft and hard limit are introduced:meta_actor_cnt_per_worker_parallelism_soft_limit
(default to 100): when the limit is exceeded,CREATE MATERIALIZED VIEW/TABLE/SINK
can still succeed but a SQL notice message will be included when the statement returns.meta_actor_cnt_per_worker_parallelism_hard_limit
(default to 400): when the limit is exceeded,CREATE MATERIALIZED VIEW/TABLE/SINK
will fail and a error message will be included when the statement returns.Notes:
CREATE MATERIALIZED VIEW/TABLE/SINK
.SET bypass_cluster_limits TO true
.